-
Notifications
You must be signed in to change notification settings - Fork 38.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding a root filesystem override for kubelet mounter #35652
Adding a root filesystem override for kubelet mounter #35652
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add the reason for this new flag/mounter parameter to the commit message body?
@@ -187,6 +187,8 @@ exit-on-lock-contention | |||
experimental-allowed-unsafe-sysctls | |||
experimental-bootstrap-kubeconfig | |||
experimental-keystone-url | |||
experimental-mounter-path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing this line will reduce your potential rebase burden, as this PR still relies on mounter-path and you already added experimental-mounter-path to known flags in #35646
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack. Done
72ded47
to
d17d01d
Compare
@@ -64,13 +66,13 @@ type Mounter struct { | |||
func (mounter *Mounter) Mount(source string, target string, fstype string, options []string) error { | |||
bind, bindRemountOpts := isBind(options) | |||
if bind { | |||
err := doMount(mounter.mounterPath, source, target, fstype, []string{"bind"}) | |||
err := doMount(mounter.mounterPath, path.Join(mounter.mounterRootfsPath, source), target, fstype, []string{"bind"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might miss something here, but source is already an absolute path?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
source
is already an absolute path in the native namespace. But from within the container, all native (root fs) paths have to be relative to the mounterRootfsPath
.
return &Mounter{ | ||
mounterPath: mounterPath, | ||
mounterPath: mounterPath, | ||
mounterRootfsPath: mounterRootfsPath, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain why this mounterRootfsPath is needed? how it works?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/
to kubelet is /rootfs
to the mounter container. So if kubelet wants to mount /foo/bar
, then it has to pass /rootfs/foo/bar
to the container.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that ok to put some comments about this somewhere in the code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. PTAL
LGTM |
d17d01d
to
c96aa14
Compare
LGTM |
Bumping priority since this is blocking 2 other PRs which have to go in asap for v1.4.x patch release |
This PR needs more work. Will pick it up tomorrow. |
c96aa14
to
0446784
Compare
Ok. I have fixed the issues with this PR. |
This is useful for supporting hostPath volumes via containerized mounters in kubelet. Signed-off-by: Vishnu kannan <vishnuk@google.com>
0446784
to
e861a57
Compare
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue |
Automatic merge from submit-queue Update GCI mounter script to run in a rkt container Depends on #35652
Automatic merge from submit-queue [Kubelet] Use the custom mounter script for Nfs and Glusterfs only This patch reduces the scope for the containerized mounter to NFS and GlusterFS on GCE + GCI clusters This patch also enabled the containerized mounter on GCI nodes Shepherding multiple PRs through the submit queue is painful. Hence I combined them into this PR. Please review each commit individually. cc @jingxu97 @saad-ali #35652 has also been reverted as part of this PR
…ck-of-#35721-kubernetes#35717-origin-release-1.4 Automatic merge from submit-queue Automated cherry pick of kubernetes#35721 kubernetes#35717 Cherry pick of kubernetes#35721 kubernetes#35717 on release-1.4. kubernetes#35721: Fixes PR kubernetes#35652 kubernetes#35717: update rkt to v1.18.0 which avoids outputting debug
Automatic merge from submit-queue [Kubelet] Use the custom mounter script for Nfs and Glusterfs only This patch reduces the scope for the containerized mounter to NFS and GlusterFS on GCE + GCI clusters This patch also enabled the containerized mounter on GCI nodes Shepherding multiple PRs through the submit queue is painful. Hence I combined them into this PR. Please review each commit individually. cc @jingxu97 @saad-ali kubernetes/kubernetes#35652 has also been reverted as part of this PR
Automatic merge from submit-queue [Kubelet] Use the custom mounter script for Nfs and Glusterfs only This patch reduces the scope for the containerized mounter to NFS and GlusterFS on GCE + GCI clusters This patch also enabled the containerized mounter on GCI nodes Shepherding multiple PRs through the submit queue is painful. Hence I combined them into this PR. Please review each commit individually. cc @jingxu97 @saad-ali kubernetes/kubernetes#35652 has also been reverted as part of this PR
Automatic merge from submit-queue [Kubelet] Use the custom mounter script for Nfs and Glusterfs only This patch reduces the scope for the containerized mounter to NFS and GlusterFS on GCE + GCI clusters This patch also enabled the containerized mounter on GCI nodes Shepherding multiple PRs through the submit queue is painful. Hence I combined them into this PR. Please review each commit individually. cc @jingxu97 @saad-ali kubernetes/kubernetes#35652 has also been reverted as part of this PR
…ootfs Automatic merge from submit-queue Adding a root filesystem override for kubelet mounter This is necessary to get hostPath volumes to work with containerized kubelet mounter
This is necessary to get hostPath volumes to work with containerized kubelet mounter
This change is